feat: Add update modal settings for major releases only#985
Conversation
- Replace boolean announceUpdates setting with dropdown (all/major/none) - Add robust semver parsing utility with comprehensive test coverage - Migrate legacy boolean values to new string preference - Show update modal only on major version bumps when 'major' option selected Closes #447
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe PR converts the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Settings as Settings UI
participant Main as Plugin (main.ts)
participant Semver as Semver Utils
User->>Settings: Select announcement option
Settings->>Main: Store announceUpdates ("all"/"major"/"none")
Note over Main: New version detected
Main->>Semver: isMajorUpdate(currentVersion, previousVersion)
Semver->>Semver: parseSemver(currentVersion)
Semver->>Semver: parseSemver(previousVersion)
Semver-->>Main: boolean (major bump detected?)
Main->>Main: Compute shouldAnnounce based on<br/>announceUpdates + isMajorUpdate result
alt announceUpdates = "all"
Main->>User: Show update notice
else announceUpdates = "major" + isMajorUpdate = true
Main->>User: Show update notice
else announceUpdates = "major" + isMajorUpdate = false
Main->>User: Skip notice
else announceUpdates = "none"
Main->>User: Skip notice
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/engine/TemplateChoiceEngine.notice.test.ts (1)
3-10: Mock value looks good; consider centralizing shared settings mock
announceUpdates: "all"correctly reflects the new union type and default behavior. As this sameDEFAULT_SETTINGSshape is duplicated across multiple tests, you might eventually extract a sharedcreateMockQuickAddSettings()/mock module to keep these in sync more easily when fields change.src/quickAddSettingsTab.ts (1)
168-189: Dropdown wiring for update announcements is correct; minor typing tweak possibleThe dropdown correctly:
- Reads the current value from
settingsStore.- Offers the three expected options (
"all","major","none").- Writes back a value consistent with
QuickAddSettings["announceUpdates"].To avoid the explicit type assertion, you could optionally centralize the allowed keys in a typed helper (e.g., a
const ANNOUNCE_OPTIONS = ["all","major","none"] as const) and narrowvaluevia that, but the current implementation is entirely fine.src/utils/semver.ts (1)
25-60: Well-implemented parsing logic with good edge case handling.The function correctly strips pre-release and build metadata, validates input thoroughly, and handles various edge cases (empty strings, invalid formats, negative numbers). The defensive programming with optional chaining and fallback values is solid.
One minor spec deviation: the official semver spec prohibits leading zeros in numeric identifiers (e.g., "01.2.3" is invalid), but this implementation would parse "01" as 1. This is likely fine for practical use since plugin versions rarely have leading zeros, and the tolerant behavior is reasonable.
If strict semver compliance is desired, you could add a leading-zero check:
const major = Number.parseInt(parts[0] ?? "", 10); const minor = Number.parseInt(parts[1] ?? "", 10); const patch = Number.parseInt(parts[2] ?? "", 10); + // Semver spec: numeric identifiers MUST NOT include leading zeroes + if (parts[0]?.startsWith("0") && parts[0] !== "0" || + parts[1]?.startsWith("0") && parts[1] !== "0" || + parts[2]?.startsWith("0") && parts[2] !== "0") { + return null; + } + // Validate that all parts are valid numbers
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/engine/CaptureChoiceEngine.notice.test.ts(1 hunks)src/engine/CaptureChoiceEngine.template-property-types.test.ts(1 hunks)src/engine/MacroChoiceEngine.notice.test.ts(1 hunks)src/engine/TemplateChoiceEngine.notice.test.ts(1 hunks)src/main.ts(3 hunks)src/quickAddSettingsTab.ts(4 hunks)src/utils/semver.test.ts(1 hunks)src/utils/semver.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/utils/semver.test.ts (1)
src/utils/semver.ts (2)
parseSemver(25-60)isMajorUpdate(76-90)
src/main.ts (2)
src/quickAddSettingsTab.ts (2)
DEFAULT_SETTINGS(56-91)QuickAddSettings(14-54)src/utils/semver.ts (1)
isMajorUpdate(76-90)
src/quickAddSettingsTab.ts (1)
src/settingsStore.ts (1)
settingsStore(7-20)
🔇 Additional comments (9)
src/engine/MacroChoiceEngine.notice.test.ts (1)
3-10: Mock default settings correctly align with newannounceUpdatestypeSwitching
announceUpdatesto"all"in the mockedDEFAULT_SETTINGSmatches the runtime union type and default, so tests stay in sync with the settings shape without changing behavior.src/engine/CaptureChoiceEngine.notice.test.ts (1)
3-10: Test settings mock updated to string-basedannounceUpdatesUsing
"all"here keeps the mock compatible with the new"all" | "major" | "none"type and with the production default.src/engine/CaptureChoiceEngine.template-property-types.test.ts (1)
8-15:announceUpdatesmock value matches new defaultSetting
announceUpdatesto"all"in this mock keeps the test harness aligned with the real settings type and default without affecting the template-property-types behavior under test.src/main.ts (2)
196-211: Settings load and boolean-to-string migration look correctMerging
loadedDataintoDEFAULT_SETTINGSand then normalizing a legacy booleanannounceUpdatesto"all"/"none"cleanly preserves existing behavior while moving to the new string union. The narrow type cast onsettingskeeps this migration local toloadSettingswithout widening the global settings type.
302-321: Update announcement logic now cleanly respects user preference and major bumpsDeriving
shouldAnnouncefromannounceUpdatesand delegating major-bump detection toisMajorUpdate(currentVersion, knownVersion)matches the new semantics:
"none": always suppress."all": preserve previous “always show on change” behavior."major": only show when the major version increases, defaulting to “show” on parse failures.Persisting
this.settings.version = currentVersionbefore returning ensures upgrades/downgrades don’t retrigger unnecessarily. This aligns well with the PR’s objective.src/quickAddSettingsTab.ts (1)
14-21:announceUpdatestype and default now match new behaviorUpdating
QuickAddSettings.announceUpdatesto the"all" | "major" | "none"union and setting the default to"all"keeps the settings contract consistent with the new announcement flow inmain.tsand maintains previous behavior for new installs.Also applies to: 56-63
src/utils/semver.test.ts (1)
1-75: Semver tests thoroughly cover expected parsing and major-update behaviorThis suite does a good job exercising
parseSemverandisMajorUpdate, including pre-release/build metadata handling, invalid inputs, null/undefined, negative numbers, major vs minor/patch bumps, downgrades, and the “err on the side of caution” path when parsing fails. It gives strong confidence that the announcement gating logic built on top will behave as intended.src/utils/semver.ts (2)
7-11: LGTM!The interface is clean and captures the essential components of a semantic version.
76-90: LGTM! Conservative fallback is well-reasoned.The major version comparison logic is correct, and the conservative fallback behavior (returning
truewhen parsing fails) is clearly documented and aligns with the goal of not missing important updates due to parsing issues. This is a sensible default that prioritizes user awareness over strict filtering.The function correctly identifies only increases in the major version number, so minor and patch updates will return
falseas expected.
# [2.8.0](2.7.0...2.8.0) (2025-11-14) ### Bug Fixes * restore Insert After matching for table separator rows ([#983](#983)) ([1393e6a](1393e6a)), closes [#970](#970) * support frontmatter tags in getFieldValues filtering ([#980](#980)) ([c9de468](c9de468)), closes [#927](#927) ### Features * Add embed replacement option for link placement ([#984](#984)) ([06a77a3](06a77a3)), closes [#893](#893) * add provider-native model discovery ([#982](#982)) ([f195c06](f195c06)) * add textarea type for advanced script settings ([#981](#981)) ([dc9a650](dc9a650)) * add update modal settings for major releases only ([#985](#985)) ([d63f8c9](d63f8c9)), closes [#447](#447) * add write to top of file switch for capture to active file ([#986](#986)) ([5361e6c](5361e6c)), closes [#248](#248) [#248](#248)
|
🎉 This PR is included in version 2.8.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
This PR implements a new setting to control when the update modal is shown, addressing issue #447. Users can now choose to see update modals only on major releases (new features, breaking changes) instead of every update.
Changes
Settings UI
Show updates on each new release(default, maintains current behavior)Show updates only on major releases (new features, breaking changes)Don't show(disables update modals)Implementation
src/utils/semver.ts): Robust semantic version parsing that handles:2.7.0)2.7.0-beta.1)2.7.0+123)nullfor graceful error handling)isMajorUpdate()function that compares major version numbersannounceUpdatesvalues:true→"all"false→"none"Testing
Technical Details
The implementation uses a dedicated utility function (
isMajorUpdate) that:This approach is more maintainable and testable than inline version parsing, and handles edge cases like pre-release versions and build metadata properly.
Migration Impact
announceUpdatessettings will be automatically migrated"all", maintaining current behaviorTesting
Closes #447
Summary by CodeRabbit
New Features
Tests